feat(Legend): add left and right options to legend.position config
#280
Conversation
|
Preview is ready. |
|
Visual Tests Report is ready. |
arteria32
changed the title
left and right options to legend.position config left and right options to legend.position config
There was a problem hiding this comment.
I think an additional field such as verticalAlign is suitable for vertical positioning of the legend (similar to horizontal align?: 'left' | 'center' | 'right';)
But this can (should) be done in a separate pr. For the legend to be positioned on the left/right, the current top-only positioning is enough - this is ok in my opinion
There was a problem hiding this comment.
But this can (should) be done in a separate pr.
OK, I will try to implement this in the next pr.
There was a problem hiding this comment.
Does there seem to be enough space from below? I would have expected to see more series in legend to take up all the available space
There was a problem hiding this comment.
I would have expected to see more series in legend to take up all the available space
I'll take a look at it.
| legend?.colorScale?.domain ?? getDomainForContinuousColorScale({series}); | ||
| } else { | ||
| height += lineHeight; | ||
| legendWidth = get(legend, 'width', DEFAULT_LEGEND_WIDTH); |
There was a problem hiding this comment.
I'm not sure if it's a good idea to use the preset fixed value setting for the legend on the right/left. As a user, I would expect that the width will be calculated depending on the width of the legend elements - that is, the series name plus the symbol. But if the width is set explicitly, then OK, you can limit it.
|
@kuzmadom |
src/constants/defaults/legend.ts
Outdated
| width: 200, | ||
| }; | ||
|
|
||
| export const DEFAULT_LEGEND_WIDTH = 80; |
There was a problem hiding this comment.
It looks like it's not in use
arteria32
force-pushed
the
feat/add-left-right-position
branch
2 times, most recently
from
f507195 to
63f5644
Compare
|
@kuzmadom |
I've fixed the legend's positioning and screenshots - hope everything should be fine now. But you need to pick up the latest version of the main branch |
ty, |
Yes, I think you should. Unfortunately, some of the snapshots were captured with existing bugs. Therefore, it happens that along with fixing a bug, you have to retake seemingly unrelated screenshots. This screenshot (in the main branch) shows that the legend ignored the specified chart margin - although it should take it into account (I think the screenshot in your branch shows the correct position). |
done |
4 participants
No description provided.